Walkthrough

Adds a new Client.$setOptions(...) API, a new client option allowQueryTimeOmitOverride, updates the omit validator to consult client options, and adds/adjusts end-to-end tests for omit and input-validation flows.

Changes

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Pay special attention to:
  • packages/orm/src/client/options.ts: verify the conditional type change for computedFields does not break type inference or introduce unintended optionality.
  • packages/orm/src/client/crud/validator/index.ts: ensure the boolean vs literal(true) distinction is applied correctly and covers all omit schema branches.
  • Tests: confirm e2e coverage matches intended runtime validation and that the new $setOptions usage is exercised where expected.

Possibly related PRs

• feat(validation): add API to suppress validation #301 — Adds client-level option setters and shifts validator behavior to read from client options (closely related API and validator changes).
• feat(orm): implement field omission #441 — Related changes to omit behaviour and client options; overlaps with validator and options edits.

Poem

🐇 I nibbled on options and stitched them anew,

A client reborn with a clearer view,
Omit rules obeyed when the flag says no,
Options swapped gently — hop, off we go!

Pre-merge checks and finishing touches

Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Use Kysely as the query builder interface for low-level database queries, avoiding raw SQL when possible

Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Claude finished @ymc9's task —— View job

Code Review for feat/rpc-api-omit

I've completed my analysis of the pull request. This change adds a new setting allowQueryTimeOmitOverride to control whether query-time omit overrides are allowed.

Todo List

• Read and analyze all modified files
• Review code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage
• Provide comprehensive feedback

Review Summary

✅ Positive Aspects

1. Clear Implementation: The feature is well-implemented with a clean separation between schema-level and validation-level changes.

2. Comprehensive Testing: The test coverage is excellent, covering both positive and negative cases for the new setting.

3. Backward Compatibility: Defaults to true to maintain existing behavior, ensuring no breaking changes.

4. Type Safety: Good integration with the existing type system and validation framework.

🔍 Code Quality & Best Practices

Good practices observed:

• Proper JSDoc documentation in packages/orm/src/client/options.ts:88-92
• Consistent naming convention (allowQueryTimeOmitOverride)
• Logical placement in the options hierarchy
• Clean validation logic in packages/orm/src/client/crud/validator/index.ts:790-796

Minor suggestions:

• The condition this.options.allowQueryTimeOmitOverride !== false is used consistently, which is correct for defaulting to true

🐛 Potential Issues

No significant bugs found, but some considerations:

1. Documentation: While the JSDoc is clear, it would be helpful to have examples in the documentation showing both true and false behaviors.

2. Error Messages: When validation fails, users get a generic validation error. Consider if a more specific error message would be helpful for this particular case.

⚡ Performance Considerations

Minimal performance impact:

• The validation check is a simple boolean comparison
• Schema caching remains effective
• No additional overhead in the hot path

🔒 Security Concerns

No security issues identified:

• The feature actually improves security by allowing stricter control over field omission
• No potential for injection or bypass attacks
• Proper validation occurs at the input validation layer

🧪 Test Coverage

Excellent test coverage:

• Tests both enabled and disabled states of the setting
• Covers various query types (create, findFirst, etc.)
• Tests the interaction with existing omit functionality
• Verifies that omit: { field: true } still works when override is disabled

Test file changes:

• tests/e2e/orm/client-api/omit.test.ts:144-161 - New test case covering the feature
• tests/e2e/orm/validation/custom-validation.test.ts - Minor adjustment for validation setup

📋 Specific Code Review Notes

packages/orm/src/client/options.ts:88-92

• Good documentation explaining the behavior when set to false
• Proper default value handling

packages/orm/src/client/crud/validator/index.ts:790-796

• Clean conditional logic
• Proper use of Zod validation types (z.boolean() vs z.literal(true))

tests/e2e/orm/client-api/omit.test.ts:144-161

• Comprehensive test covering the main use case
• Good use of toBeRejectedByValidation() matcher
• Tests both rejection and acceptance cases

🎯 Recommendations

1. Documentation Enhancement: Consider adding usage examples to the main documentation showing when and why to use this setting.

2. Error Message Improvement: Consider providing a more specific error message when omit override is attempted but not allowed.

3. Type Definitions: The implementation correctly handles the TypeScript types, maintaining type safety.

Overall Assessment

This is a well-implemented feature that adds useful functionality for controlling field omission behavior. The code quality is high, test coverage is comprehensive, and the implementation follows established patterns in the codebase. The feature enhances security and control without introducing breaking changes.

Recommendation: ✅ Approve with minor suggestions above

Claude finished @ymc9's task —— View job

Code Review for feat/rpc-api-omit

I've completed my analysis of the pull request. This change adds a new setting allowQueryTimeOmitOverride to control whether query-time omit overrides are allowed.

Todo List

• Read and analyze all modified files
• Review code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage
• Provide comprehensive feedback

Review Summary

✅ Positive Aspects

1. Clear Implementation: The feature is well-implemented with a clean separation between schema-level and validation-level changes.

2. Comprehensive Testing: The test coverage is excellent, covering both positive and negative cases for the new setting.

3. Backward Compatibility: Defaults to true to maintain existing behavior, ensuring no breaking changes.

4. Type Safety: Good integration with the existing type system and validation framework.

🔍 Code Quality & Best Practices

Good practices observed:

• Proper JSDoc documentation in packages/orm/src/client/options.ts:88-92
• Consistent naming convention (allowQueryTimeOmitOverride)
• Logical placement in the options hierarchy
• Clean validation logic in packages/orm/src/client/crud/validator/index.ts:790-796

Minor suggestions:

• The condition this.options.allowQueryTimeOmitOverride !== false is used consistently, which is correct for defaulting to true

🐛 Potential Issues

No significant bugs found, but some considerations:

1. Documentation: While the JSDoc is clear, it would be helpful to have examples in the documentation showing both true and false behaviors.

2. Error Messages: When validation fails, users get a generic validation error. Consider if a more specific error message would be helpful for this particular case.

⚡ Performance Considerations

Minimal performance impact:

• The validation check is a simple boolean comparison
• Schema caching remains effective
• No additional overhead in the hot path

🔒 Security Concerns

No security issues identified:

• The feature actually improves security by allowing stricter control over field omission
• No potential for injection or bypass attacks
• Proper validation occurs at the input validation layer

🧪 Test Coverage

Excellent test coverage:

• Tests both enabled and disabled states of the setting
• Covers various query types (create, findFirst, etc.)
• Tests the interaction with existing omit functionality
• Verifies that omit: { field: true } still works when override is disabled

Test file changes:

• tests/e2e/orm/client-api/omit.test.ts:144-161 - New test case covering the feature
• tests/e2e/orm/validation/custom-validation.test.ts - Minor adjustment for validation setup

📋 Specific Code Review Notes

packages/orm/src/client/options.ts:88-92

• Good documentation explaining the behavior when set to false
• Proper default value handling

packages/orm/src/client/crud/validator/index.ts:790-796

• Clean conditional logic
• Proper use of Zod validation types (z.boolean() vs z.literal(true))

tests/e2e/orm/client-api/omit.test.ts:144-161

• Comprehensive test covering the main use case
• Good use of toBeRejectedByValidation() matcher
• Tests both rejection and acceptance cases

🎯 Recommendations

1. Documentation Enhancement: Consider adding usage examples to the main documentation showing when and why to use this setting.

2. Error Message Improvement: Consider providing a more specific error message when omit override is attempted but not allowed.

3. Type Definitions: The implementation correctly handles the TypeScript types, maintaining type safety.

Overall Assessment

This is a well-implemented feature that adds useful functionality for controlling field omission behavior. The code quality is high, test coverage is comprehensive, and the implementation follows established patterns in the codebase. The feature enhances security and control without introducing breaking changes.

Recommendation: ✅ Approve with minor suggestions above